-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Asymmetric line flow (Refined); READY FOR REVIEW !!! #789
base: develop
Are you sure you want to change the base?
Conversation
0002caf
to
0e224c5
Compare
Hi @JesseJenkins and @lbonaldo , I have pushed all the modifications to the code base for this PR. Few things to note: First of all, as you will see, it's way more lengthy and complicated than what Jesse suggested to me previously. I first tried with the simpler version. But, was getting bidirectional flows on the asymmetric lines with Qian's example case even when we restricted the flow limits to zero in one of the directions for the asymmetric lines. With some pondering, I realized that I was doing wrong indexing on the constraints and decision variables. So, I had to do a bit more digging and refining the code. The fundamental reasons for which the code is so much lengthier are: 1) I have both symmetric as well as asymmetric lines in a system and 2) In order to maintain backward compatibility with the existing example cases (As you will observe, I had to go to lengths to split the data-frame into two parts and assign separate variables and constraint names so that things stay clean and correct. I could not find an easier way to do this. I'll highly appreciate any feedback on this). We tested with @qluo0320github 's example cases and it seems the results are sensible (?) However, I still get discrepancies when I benchmark against an actually symmetric system versus a "simulated" symmetric system (in which the asymmetric lines have the same flow limits, loss percentages, and transmission buildout in both directions). I am attaching hereby all the relevant flow comparison I scrutinized the code several times, but couldn't spot any flaw so far. I would appreciate if you could point some obvious bug (if at all) that I might have missed. Also, I would appreciate taking a look at the quadratic loss formulation. I believe I did it right, but am not totally certain. Thank you so much !!! I have also updated most of the doc pages; I will wrap up parts of the transmission.jl doc page and make another push to close this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
JuliaFormatter
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 280 to 282 in 7d90bd9
if setup["asymmetrical_trans_flow_limit"] ==1 |
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 289 to 290 in 7d90bd9
cMaxFlow_out_asym[l = 1:L_asym, t = 1:T], vFLOW[(l+L_sym), t] <= EP[:eAvail_Trans_Cap_Pos][l] #Change these with Auxiliary | |
cMaxFlow_in_asym[l = 1:L_asym, t = 1:T], vFLOW[(l+L_sym), t] >= -EP[:eAvail_Trans_Cap_Neg][l] #Change these with Auxiliary |
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 294 to 297 in 7d90bd9
begin | |
cMaxFlow_out[l = 1:L, t = 1:T], vFLOW[l, t] <= EP[:eAvail_Trans_Cap][l] | |
cMaxFlow_in[l = 1:L, t = 1:T], vFLOW[l, t] >= -EP[:eAvail_Trans_Cap][l] | |
end) |
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
if setup["asymmetrical_trans_flow_limit"] ==1 |
[JuliaFormatter] reported by reviewdog 🐶
inputs["pPercent_Loss_Pos"][l] * (vTAUX_POS_ASYM[l, t]) + inputs["pPercent_Loss_Neg"][l] * (vTAUX_NEG_ASYM[l, t]) |
[JuliaFormatter] reported by reviewdog 🐶
vTAUX_POS_ASYM[l, t] - vTAUX_NEG_ASYM[l, t] == vFLOW[(l+L_sym), t] |
[JuliaFormatter] reported by reviewdog 🐶
vTAUX_POS_ASYM[l, t] + vTAUX_NEG_ASYM[l, t] <= min(EP[:eAvail_Trans_Cap_Pos][l], EP[:eAvail_Trans_Cap_Neg][l]) |
[JuliaFormatter] reported by reviewdog 🐶
vTAUX_NEG[l, t] <= EP[:eAvail_Trans_Cap][l] - vPROD_TRANSCAP_ON[l, t] |
[JuliaFormatter] reported by reviewdog 🐶
vTAUX_NEG_ASYM[l, t] <= EP[:eAvail_Trans_Cap_Neg][l] - vPROD_TRANSCAP_ON_NEG_ASYM[l, t] |
[JuliaFormatter] reported by reviewdog 🐶
vTAUX_POS_ASYM[l, t] <= EP[:eAvail_Trans_Cap_Pos][l] - vPROD_TRANSCAP_ON_POS_ASYM[l, t] |
[JuliaFormatter] reported by reviewdog 🐶
(1 - vTAUX_POS_ON_POS_ASYM[l, t]) * inputs["pTrans_Max_Possible_Pos"][l] |
[JuliaFormatter] reported by reviewdog 🐶
(1 - vTAUX_POS_ON_NEG_ASYM[l, t]) * inputs["pTrans_Max_Possible_Neg"][l] |
[JuliaFormatter] reported by reviewdog 🐶
vTAUX_NEG[l, t] <= EP[:eAvail_Trans_Cap][l] - vPROD_TRANSCAP_ON[l, t] |
[JuliaFormatter] reported by reviewdog 🐶
if setup["asymmetrical_trans_flow_limit"] ==1 |
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 470 to 471 in 7d90bd9
sum((2 * s - 1) * (inputs["pTrans_Max_Possible"][l] / TRANS_LOSS_SEGS) * | |
vTAUX_POS[l, s, t] for s in 1:TRANS_LOSS_SEGS)) + |
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 473 to 474 in 7d90bd9
sum((2 * s - 1) * (inputs["pTrans_Max_Possible"][l] / TRANS_LOSS_SEGS) * | |
vTAUX_NEG[l, s, t] for s in 1:TRANS_LOSS_SEGS))) |
[JuliaFormatter] reported by reviewdog 🐶
sum(vTAUX_POS[l, s, t] for s in 1:TRANS_LOSS_SEGS) - vTAUX_POS[l, 0, t] == |
[JuliaFormatter] reported by reviewdog 🐶
sum(vTAUX_NEG[l, s, t] for s in 1:TRANS_LOSS_SEGS) - vTAUX_NEG[l, 0, t] == |
[JuliaFormatter] reported by reviewdog 🐶
cTAuxOrderPos2[l in LOSS_LINES_SYM, s = 1:(TRANS_LOSS_SEGS - 1), t = 1:T], |
[JuliaFormatter] reported by reviewdog 🐶
cTAuxOrderNeg2[l in LOSS_LINES_SYM, s = 1:(TRANS_LOSS_SEGS - 1), t = 1:T], |
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 548 to 549 in 7d90bd9
sum((2 * s - 1) * (inputs["pTrans_Max_Possible_Pos"][l] / TRANS_LOSS_SEGS) * | |
vTAUX_POS_ASYM[l, s, t] for s in 1:TRANS_LOSS_SEGS)) + |
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 551 to 552 in 7d90bd9
sum((2 * s - 1) * (inputs["pTrans_Max_Possible_Neg"][l] / TRANS_LOSS_SEGS) * | |
vTAUX_NEG_ASYM[l, s, t] for s in 1:TRANS_LOSS_SEGS))) |
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 558 to 559 in 7d90bd9
sum(vTAUX_POS_ASYM[l, s, t] for s in 1:TRANS_LOSS_SEGS) - vTAUX_POS_ASYM[l, 0, t] == | |
vFLOW[(l+L_sym), t] |
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 561 to 562 in 7d90bd9
sum(vTAUX_NEG_ASYM[l, s, t] for s in 1:TRANS_LOSS_SEGS) - vTAUX_NEG_ASYM[l, 0, t] == | |
-vFLOW[(l+L_sym), t] |
[JuliaFormatter] reported by reviewdog 🐶
cTAuxMaxNeg_asym[l in LOSS_LINES_ASYM, s = 1:TRANS_LOSS_SEGS, t = 1:T], |
[JuliaFormatter] reported by reviewdog 🐶
cTAuxOrderPos1_asym[l in LOSS_LINES_ASYM, s = 1:TRANS_LOSS_SEGS, t = 1:T], |
[JuliaFormatter] reported by reviewdog 🐶
cTAuxOrderNeg1_asym[l in LOSS_LINES_ASYM, s = 1:TRANS_LOSS_SEGS, t = 1:T], |
[JuliaFormatter] reported by reviewdog 🐶
cTAuxOrderPos2_asym[l in LOSS_LINES_ASYM, s = 1:(TRANS_LOSS_SEGS - 1), t = 1:T], |
[JuliaFormatter] reported by reviewdog 🐶
cTAuxOrderNeg2_asym[l in LOSS_LINES_ASYM, s = 1:(TRANS_LOSS_SEGS - 1), t = 1:T], |
[JuliaFormatter] reported by reviewdog 🐶
inputs["pTrans_Max_Possible_Pos"][l] * (1 - vTAUX_POS_ON_ASYM[l, 1, t]) |
[JuliaFormatter] reported by reviewdog 🐶
inputs["pTrans_Max_Possible_Neg"][l] * (1 - vTAUX_NEG_ON_ASYM[l, 1, t]) |
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 625 to 626 in 7d90bd9
sum((2 * s - 1) * (inputs["pTrans_Max_Possible"][l] / TRANS_LOSS_SEGS) * | |
vTAUX_POS[l, s, t] for s in 1:TRANS_LOSS_SEGS)) + |
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 628 to 629 in 7d90bd9
sum((2 * s - 1) * (inputs["pTrans_Max_Possible"][l] / TRANS_LOSS_SEGS) * | |
vTAUX_NEG[l, s, t] for s in 1:TRANS_LOSS_SEGS))) |
[JuliaFormatter] reported by reviewdog 🐶
sum(vTAUX_POS[l, s, t] for s in 1:TRANS_LOSS_SEGS) - vTAUX_POS[l, 0, t] == |
[JuliaFormatter] reported by reviewdog 🐶
sum(vTAUX_NEG[l, s, t] for s in 1:TRANS_LOSS_SEGS) - vTAUX_NEG[l, 0, t] == |
[JuliaFormatter] reported by reviewdog 🐶
cTAuxOrderPos2[l in LOSS_LINES, s = 1:(TRANS_LOSS_SEGS - 1), t = 1:T], |
[JuliaFormatter] reported by reviewdog 🐶
cTAuxOrderNeg2[l in LOSS_LINES, s = 1:(TRANS_LOSS_SEGS - 1), t = 1:T], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
JuliaFormatter
[JuliaFormatter] reported by reviewdog 🐶
cTAuxOrderNeg2_asym[l in LOSS_LINES_ASYM, s = 1:(TRANS_LOSS_SEGS - 1), t = 1:T], |
[JuliaFormatter] reported by reviewdog 🐶
inputs["pTrans_Max_Possible_Pos"][l] * (1 - vTAUX_POS_ON_ASYM[l, 1, t]) |
[JuliaFormatter] reported by reviewdog 🐶
inputs["pTrans_Max_Possible_Neg"][l] * (1 - vTAUX_NEG_ON_ASYM[l, 1, t]) |
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 628 to 629 in 146916a
sum((2 * s - 1) * (inputs["pTrans_Max_Possible"][l] / TRANS_LOSS_SEGS) * | |
vTAUX_POS[l, s, t] for s in 1:TRANS_LOSS_SEGS)) + |
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 631 to 632 in 146916a
sum((2 * s - 1) * (inputs["pTrans_Max_Possible"][l] / TRANS_LOSS_SEGS) * | |
vTAUX_NEG[l, s, t] for s in 1:TRANS_LOSS_SEGS))) |
[JuliaFormatter] reported by reviewdog 🐶
sum(vTAUX_POS[l, s, t] for s in 1:TRANS_LOSS_SEGS) - vTAUX_POS[l, 0, t] == |
[JuliaFormatter] reported by reviewdog 🐶
sum(vTAUX_NEG[l, s, t] for s in 1:TRANS_LOSS_SEGS) - vTAUX_NEG[l, 0, t] == |
[JuliaFormatter] reported by reviewdog 🐶
cTAuxOrderPos2[l in LOSS_LINES, s = 1:(TRANS_LOSS_SEGS - 1), t = 1:T], |
[JuliaFormatter] reported by reviewdog 🐶
cTAuxOrderNeg2[l in LOSS_LINES, s = 1:(TRANS_LOSS_SEGS - 1), t = 1:T], |
… asymmetrical flows
…olumns for bidirectional asymmetric flow
…refined write_expansion
1d3943a
to
6fc5330
Compare
- Modified `transmission.jl`, `investment_transmission.jl`, `load_network_data.jl`, | ||
`write_transmission_flows.jl`, `write_transmision_losses.jl`, and `write_network_expansion.jl` | ||
for implementing asymmetric bidirectional flows. (#789) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part should go under the ##Unreleased
section above as it belongs to a new release.
@@ -0,0 +1,32 @@ | |||
# Three Zones |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we decide to keep this example, we might need to this description.
UCommit: 2 # Unit committment of thermal power plants; 0 = not active; 1 = active using integer clestering; 2 = active using linearized clustering | ||
TimeDomainReduction: 1 # Time domain reduce (i.e. cluster) inputs based on Demand_data.csv, Generators_variability.csv, and Fuels_data.csv; 0 = not active (use input data as provided); 0 = active (cluster input data, or use data that has already been clustered) | ||
OutputFullTimeSeries: 1 | ||
asymmetrical_trans_flow_limit: 1 # Switch to indicate if asymmetrical bidirectional lines are included in the system or not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By convention, we use PascalCase (or UpperCamelCase) for setting keys: AsymmetricalTransFlowLimit
.
ME,z3,4,1,2,1,2000,4000,MA_to_CT,123.0584,0.012305837,0.12305837,1206,2412,12075,0.95,0,0 | ||
,,5,1,3,1,1000,800,MA_to_ME,196.5385,0.019653847,0.19653847,1926,963,19200,0.95,0,0 | ||
,,6,2,3,1,500,1200,CT_to_ME,196.5385,0.019653847,0.19653847,1206,603,12000,0.95,0,0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the Network_Lines
column should be [1, 2, 3, 4, 5]
here. I'm getting an error when trying to run this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm encountering the following error message when trying to run this example:
Reading Input CSV Files
ERROR: ArgumentError: column name :Asymmetrical not found in the data frame
@@ -37,7 +37,9 @@ function default_settings() | |||
"ResourcePoliciesFolder" => "policy_assignments", | |||
"SystemFolder" => "system", | |||
"PoliciesFolder" => "policies", | |||
"ObjScale" => 1) | |||
"ObjScale" => 1, | |||
"asymmetrical_trans_flow_limit" => 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, we typically use PascalCase (or UpperCamelCase) for setting keys.
if setup["asymmetrical_trans_flow_limit"] == 1 | ||
L_asym = length(filtered_vector(network_var, :Asymmetrical, 1, :Network_Lines)) #Number of asymmetrical lines | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line will throw an error if ["asymmetrical_trans_flow_limit"] == 1
but the :Asymmetrical
column is not found in the Network.csv
file. We could add a validation to ensure that:
- If the user sets ["asymmetrical_trans_flow_limit"] == 1
- Then a corresponding set of additional columns must be provided.
for i in EXPANSION_LINES_ASYM | ||
transcap_pos[i] = value.(EP[:vNEW_TRANS_CAP_Pos][i]) | ||
transcap_neg[i] = value.(EP[:vNEW_TRANS_CAP_Neg][i]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for i in EXPANSION_LINES_ASYM | |
transcap_pos[i] = value.(EP[:vNEW_TRANS_CAP_Pos][i]) | |
transcap_neg[i] = value.(EP[:vNEW_TRANS_CAP_Neg][i]) | |
for i in eachindex(EXPANSION_LINES_ASYM) | |
asym_line_index = EXPANSION_LINES_ASYM[i] | |
transcap_pos[i] = value.(EP[:vNEW_TRANS_CAP_Pos][asym_line_index]) | |
transcap_neg[i] = value.(EP[:vNEW_TRANS_CAP_Neg][asym_line_index]) |
@@ -122,14 +122,14 @@ function write_outputs(EP::Model, path::AbstractString, setup::Dict, inputs::Dic | |||
println(elapsed_time_flows) | |||
end | |||
|
|||
if output_settings_d["WriteTransmissionLosses"] | |||
#=if output_settings_d["WriteTransmissionLosses"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we uncomment this part?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First round of review. Thanks @sambuddhac for this PR. In your opinion, which example case should we keep? The branch is now rebased with the latest develop.
L_sym = inputs["L_sym"] | ||
L_asym = inputs["L_asym"] #Number of transmission lines with different capacities in two directions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L_sym doesn't seem to be used
L_sym = inputs["L_sym"] # Number of transmission lines with symmetrical bidirectional flow | ||
L_asym = inputs["L_asym"] #Number of transmission lines with different capacities in two directions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two variables do not seem to be used.
SYMMETRIC_LINE_INDEX = inputs["symmetric_line_index"] | ||
ASYMMETRIC_LINE_INDEX = inputs["asymmetric_line_index"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that the convention for inputs
keys is to use capitalized words separated by underscores. What do you think about applying this convention to these two keys as well?
|
||
if setup["asymmetrical_trans_flow_limit"] == 1 | ||
# Transmission capacity of the network for asymmetrical lines; return direction(in MW) | ||
inputs_nw["pTrans_Max_Possible_Neg"] = inputs_nw["pTrans_Max_Neg"] = to_floats(:Line_Max_Flow_MW_Neg) / scale_factor # convert to GW |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another small suggestion: we usually place the units at the end of the key, e.g., Inv_Cost_Per_MWyr
, Line_Max_Flow_MW, etc
L_sym = inputs["L_sym"] | ||
L_asym = inputs["L_asym"] #Number of transmission lines with different capacities in two directions | ||
L = inputs["L"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These three variables do not seem to be used.
|
||
if setup["NetworkExpansion"] == 1 | ||
# Network lines and zones that are expandable have non-negative maximum reinforcement inputs | ||
inputs_nw["EXPANSION_LINES"] = findall(inputs_nw["pMax_Line_Reinforcement"] .>= 0) | ||
inputs_nw["NO_EXPANSION_LINES"] = findall(inputs_nw["pMax_Line_Reinforcement"] .< 0) | ||
inputs_nw["EXPANSION_LINES"] = findall(inputs_nw["pMax_Line_Reinforcement"] .> 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could do the intersection with SYMMETRIC_LINE_INDEX
here to avoid needing to do it throughout the rest of the code.
#scale_factor # Convert to GW | ||
inputs_nw["pLine_Max_Flow_Possible_MW_Neg"] = to_floats(:Line_Max_Flow_Possible_MW_Neg) / | ||
scale_factor # Convert to GW | ||
end | ||
end | ||
|
||
## Sets and indices for transmission losses and expansion | ||
inputs_nw["TRANS_LOSS_SEGS"] = setup["Trans_Loss_Segments"] # Number of segments used in piecewise linear approximations quadratic loss functions | ||
inputs_nw["LOSS_LINES"] = findall(inputs_nw["pTrans_Loss_Coef"] .!= 0) # Lines for which loss coefficients apply (are non-zero); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as below, we could do the intersection with SYMMETRIC_LINE_INDEX here instead of throughout the rest of the code.
if NetworkExpansion == 1 | ||
# Network lines and zones that are expandable have non-negative maximum reinforcement inputs | ||
EXPANSION_LINES = inputs["EXPANSION_LINES"] | ||
EXPANSION_LINES_ASYM = inputs["EXPANSION_LINES_ASYM"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two variables do not seem to be used.
Description
This PR attempts to introduce bidirectional asymmetric transmission lines into the system. Such lines have different values of MW flow limits, loss percentage, and capacity expansion limits along the two directions. While doing so, this PR also ensures that there are also symmetric lines present in the system along with the asymmetric lines and modifies the code in such a way that backwards compatibility is maintained with the existing cases.
What type of PR is this? (check all applicable)
Related Tickets & Documents
Checklist
How this can be tested
Post-approval checklist for GenX core developers
After the PR is approved